Skip to content

chore: remove extra MLS verification transactions on epoch changes [WPB-23368] #3857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Garzas wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

chore: remove extra MLS verification transactions on epoch changes [WPB-23368] #3857

Garzas wants to merge 2 commits into from

Conversation

@Garzas
Copy link
Contributor

@Garzas Garzas commented Feb 10, 2026 •
edited by github-actions bot
Loading

https://wearezeta.atlassian.net/browse/WPB-23368

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

  • Verification refresh during MLS epoch-change flow was opening additional CC transactions.
  • During batch decrypt, those extra transactions could be blocked by ongoing decrypt work, causing delays and noisy waiting logs.
  • Member verification status retrieval used a mixed access pattern, leading to duplicated read logic.

Causes (Optional)

  • Verification checks in epoch/conversation flows were still tied to transaction-based access.
  • Read-only verification/identity checks were not consistently routed through the same non-blocking path.

Solutions

  • Removed extra transaction usage from epoch-change and conversation verification refresh flows.
  • Switched verification state reads to the read-only client path.
  • Standardized member-identity retrieval by passing the client explicitly to repository methods, instead of using transaction context/provider inside the repository.

@Garzas Garzas self-assigned this Feb 10, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2026 •
edited
Loading

Test Results

0 tests   - 4 331   0 ✅  - 4 214   0s ⏱️ - 5m 50s
0 suites  -   725   0 💤  -   117 
0 files    -   725   0 ❌ ±    0 

Results for commit 5229c1c. ± Comparison against base commit 3e70856.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2026 •
edited
Loading

🐰 Bencher Report

Branchchore/mls-verification-without-extra-transactions
Testbedubuntu-latest

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
BenchmarkLatencymicroseconds (µs)
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInFiles📈 view plot
⚠️ NO THRESHOLD		717.05 µs
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInMemory📈 view plot
⚠️ NO THRESHOLD		350,551.75 µs
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.messageInsertionBenchmark📈 view plot
⚠️ NO THRESHOLD		1,371,838.71 µs
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.queryMessagesBenchmark📈 view plot
⚠️ NO THRESHOLD		21,156.60 µs
🐰 View full continuous benchmarking report in Bencher

MohamadJaara
MohamadJaara reviewed Feb 10, 2026
View reviewed changes
@Garzas @MohamadJaara
Update gradle/libs.versions.toml
5229c1c 
Co-authored-by: Mohamad Jaara <mohamad.jaara@wire.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 11, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.57%. Comparing base (8478f6d) to head (5229c1c).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
...tlin/com/wire/kalium/cryptography/MLSClientImpl.kt 0.00% 5 Missing ⚠️
...in/com/wire/kalium/logic/feature/user/UserScope.kt 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3857      +/-   ##
===========================================
- Coverage    59.58%   59.57%   -0.01%     
===========================================
  Files         1899     1900       +1     
  Lines        59335    59476     +141     
  Branches      6430     6451      +21     
===========================================
+ Hits         35356    35435      +79     
- Misses       21018    21080      +62     
  Partials      2961     2961
Files with missing lines Coverage Δ
...n/kotlin/com/wire/kalium/cryptography/MLSClient.kt 50.74% <ø> (ø)
...gic/data/conversation/MLSConversationRepository.kt 76.42% <100.00%> (-0.58%) ⬇️
...e/FetchConversationMLSVerificationStatusUseCase.kt 100.00% <100.00%> (+18.18%) ⬆️
.../e2ei/usecase/FetchMLSVerificationStatusUseCase.kt 87.75% <100.00%> (+0.65%) ⬆️
...secase/GetMembersE2EICertificateStatusesUseCase.kt 80.95% <100.00%> (+8.22%) ⬆️
...rveE2EIConversationsVerificationStatusesUseCase.kt 100.00% <100.00%> (+15.38%) ⬆️
...in/com/wire/kalium/logic/feature/user/UserScope.kt 0.00% <0.00%> (ø)
...tlin/com/wire/kalium/cryptography/MLSClientImpl.kt 43.18% <0.00%> (-1.71%) ⬇️

... and 34 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8478f6d...5229c1c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sbakhtiarov
sbakhtiarov approved these changes Feb 11, 2026
View reviewed changes
core/cryptography/src/commonMain/kotlin/com/wire/kalium/cryptography/MLSClientImpl.kt
}
}

override suspend fun isGroupVerified(groupId: MLSGroupId): E2EIConversationState {
Copy link
Contributor

@sbakhtiarov sbakhtiarov Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment: either move the logic for checking the group status in this method (ccGroupStatus == VerificationStatus.VERIFIED) and return boolean or change the name to getGroupState().
Otherwise the name / return type do not match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MohamadJaara MohamadJaara MohamadJaara left review comments

@sbakhtiarov sbakhtiarov sbakhtiarov approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@Garzas Garzas

Labels

🚨 Potential breaking changes 👕 size: M type: chore 🧹

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Garzas @codecov-commenter @sbakhtiarov @MohamadJaara